Skip to content

Improve join order for RangeAnalysis::boundedPhi#8393

Closed
smowton wants to merge 3 commits into
github:mainfrom
smowton:smowton/fix/bounded-phi-join-order
Closed

Improve join order for RangeAnalysis::boundedPhi#8393
smowton wants to merge 3 commits into
github:mainfrom
smowton:smowton/fix/bounded-phi-join-order

Conversation

@smowton

@smowton smowton commented Mar 10, 2022

Copy link
Copy Markdown
Contributor

No description provided.

@smowton smowton requested a review from a team as a code owner March 10, 2022 15:10
@github-actions github-actions Bot added the Java label Mar 10, 2022
@smowton smowton added the no-change-note-required This PR does not need a change note label Mar 10, 2022
Reason reason
) {
forex(SsaVariable inp, SsaReadPositionPhiInputEdge edge | edge.phiInput(phi, inp) |
// Forcibly split into an existence and a forall stage instead of using forex because otherwise

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am surprised to see that this doesn't use the "ranking approach" as in e.g. the modulus library. Replacing recursion through forall/forex with recursion over ranked indices typically performs much better.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like #8405 ? Which do you prefer?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I prefer that PR over this one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this one also appears to have a significant slow-down on palatable/lambda).

@smowton

smowton commented Mar 14, 2022

Copy link
Copy Markdown
Contributor Author

Dropped in favour of #8405

@smowton smowton closed this Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Java no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants